fix($sniffer): don't use history.pushState for sandboxed Chrome apps #15021
Conversation
Check in $sniffer if operating in a sandboxed Chrome app, which does not have access to chrome.app.runtime like other apps, but also does not have access to history.pushState. If inside a sandboxed Chrome app, do not try and use history.pushState, else an error will occur. See angular#11932 and angular#13945 for previous work.
|
😐 build says Karma tests failed, but I don't see any failed tests. |
| isChromePackagedApp = | ||
| $window.chrome && | ||
| ($window.chrome.app && $window.chrome.app.runtime || | ||
| !$window.chrome.app && $window.chrome.runtime && $window.chrome.runtime.id), |
There was a problem hiding this comment.
I don't think this check can distinguish sandboxed apps from extensions. We need a check that can reliably detect sandboxed apps only.
There was a problem hiding this comment.
It should be able to. Extensions should always have a $window.chrome.app object. It may not have much in the object (there's no runtime child like with apps), but it will be there nonetheless, whereas with sandbox apps the object does not exist at all, because the APIs are not provided.
I'll try and investigate a better way of detecting this, but this seems to be the only way I can find that does not use APIs that would require an extension to have non-generic permissions for that API.
There was a problem hiding this comment.
Ah, I didn't pay much attention to the !$window.chrome.app check. It should be good then.
I wish there were a more straight-forward way to detect chrome packaged apps 😞
There was a problem hiding this comment.
Sorry for the delay. I've waited for advice, but it appears there really is no other way of checking at the moment.
|
It basically LGTM, but it would be awesome if there was a more straight-forward way for detecting Chrome Packaged Apps. I'll let you look into it for a few days if you feel like it 😄 - feel free to ping me when you're done investigating. |
…aged Apps While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt accessing `history.pushState` as "normal" CPAs, they can't be detected in the same way, as they do not have access to the same APIs. Previously, due to their differences from normal CPAs, `$sniffer` would fail to detect sandboxed CPAs and incorrectly assume `history.pushState` is available (which resulted in an error being thrown). This commit fixes the detection of sandboxed CPAs in `$sniffer`. See #11932 and #13945 for previous work. Closes #15021
…aged Apps While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt accessing `history.pushState` as "normal" CPAs, they can't be detected in the same way, as they do not have access to the same APIs. Previously, due to their differences from normal CPAs, `$sniffer` would fail to detect sandboxed CPAs and incorrectly assume `history.pushState` is available (which resulted in an error being thrown). This commit fixes the detection of sandboxed CPAs in `$sniffer`. See angular#11932 and angular#13945 for previous work. Closes angular#15021
…aged Apps While sandboxed Chrome Packaged Apps (CPAs) have the same restrictions wrt accessing `history.pushState` as "normal" CPAs, they can't be detected in the same way, as they do not have access to the same APIs. Previously, due to their differences from normal CPAs, `$sniffer` would fail to detect sandboxed CPAs and incorrectly assume `history.pushState` is available (which resulted in an error being thrown). This commit fixes the detection of sandboxed CPAs in `$sniffer`. See angular#11932 and angular#13945 for previous work. Closes angular#15021
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
Bug fix
What is the current behavior? (You can also link to an open issue here)
When using Angular in a sandboxed Chrome app, errors occur when attempting to call
history.pushState(), which is not available.What is the new behavior (if this is a feature change)?
Similar to #11932 and building on the code in #13945, check if inside a sandboxed app, and substitute
history.pushState()with a no-op.Does this PR introduce a breaking change?
No
Please check if the PR fulfills these requirements
Other information:
Check in $sniffer if operating in a sandboxed Chrome app, which does not
have access to chrome.app.runtime like other apps, but also does not
have access to history.pushState. If inside a sandboxed Chrome app, do
not try and use history.pushState, else an error will occur. See #11932
and #13945 for previous work.